pull: don't use static deltas if archive repo

In https://github.com/ostreedev/ostree/pull/408, we disabled the use of
static deltas when mirroring. Later,
https://github.com/ostreedev/ostree/pull/506 loosened this up again so
that we could use static deltas when mirroring into bare{-user} repos.

However, the issue which originally spurrred #408 is even more generic
than that: we want to avoid static deltas for any archive repo, not just
when doing a mirror pull. This patch tightens this up, and also
relocates the decision code to make it easier to read.

Closes: #715
Approved by: cgwalters

diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c
index 7cbe8f92da65a180d0664c6f68c7fb97e372a134..60eb214ee7683a125f49ef152b8824fda190c818 100644 (file)
--- a/src/libostree/ostree-repo-pull.c
+++ b/src/libostree/ostree-repo-pull.c
@@ -2542,7 +2542,6 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
   gboolean opt_gpg_verify_set = FALSE;
   gboolean opt_gpg_verify_summary_set = FALSE;
   const char *url_override = NULL;
-  gboolean mirroring_into_archive;
   gboolean inherit_transaction = FALSE;
   int i;
 
@@ -2581,6 +2580,7 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
     g_return_val_if_fail (dirs_to_pull[i][0] == '/', FALSE);
 
   g_return_val_if_fail (!(disable_static_deltas && pull_data->require_static_deltas), FALSE);
+
   /* We only do dry runs with static deltas, because we don't really have any
    * in-advance information for bare fetches.
    */
@@ -2591,8 +2591,6 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
   pull_data->is_untrusted = (flags & OSTREE_REPO_PULL_FLAGS_UNTRUSTED) > 0;
   pull_data->cancellable = cancellable ? g_object_ref (cancellable) : NULL;
 
-  mirroring_into_archive = pull_data->is_mirror && self->mode == OSTREE_REPO_MODE_ARCHIVE_Z2;
-
   if (error)
     pull_data->async_error = &pull_data->cached_async_error;
   else
@@ -2862,6 +2860,23 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
   if (pull_data->remote_repo_local && !pull_data->require_static_deltas)
     disable_static_deltas = TRUE;
 
+  /* We can't use static deltas if pulling into an archive-z2 repo. */
+  if (self->mode == OSTREE_REPO_MODE_ARCHIVE_Z2)
+    {
+      if (pull_data->require_static_deltas)
+        {
+          g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED,
+                               "Can't use static deltas in an archive repo");
+          goto out;
+        }
+      disable_static_deltas = TRUE;
+    }
+
+  /* It's not efficient to use static deltas if all we want is the commit
+   * metadata. */
+  if (pull_data->is_commit_only)
+    disable_static_deltas = TRUE;
+
   pull_data->static_delta_superblocks = g_ptr_array_new_with_free_func ((GDestroyNotify)g_variant_unref);
 
   {
@@ -3148,7 +3163,7 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
                                     &from_revision, error))
         goto out;
 
-      if (!(disable_static_deltas || mirroring_into_archive || pull_data->is_commit_only) &&
+      if (!disable_static_deltas &&
           (from_revision == NULL || g_strcmp0 (from_revision, to_revision) != 0))
         {
           g_autofree char *delta_name =

diff --git a/tests/pull-test.sh b/tests/pull-test.sh
index 693eac06ebf5c08cfdbf2542f63a8ae84184abdf..f61760797fd90cd8762c4b088251bb0775581b76 100644 (file)
--- a/tests/pull-test.sh
+++ b/tests/pull-test.sh
@@ -35,7 +35,7 @@ function verify_initial_contents() {
     assert_file_has_content baz/cow '^moo$'
 }
 
-echo "1..14"
+echo "1..15"
 
 # Try both syntaxes
 repo_init
@@ -69,6 +69,16 @@ ${CMD_PREFIX} ostree --repo=mirrorrepo pull --mirror origin main
 ${CMD_PREFIX} ostree --repo=mirrorrepo fsck
 echo "ok pull mirror (should not apply deltas)"
 
+cd ${test_tmpdir}
+if ${CMD_PREFIX} ostree --repo=mirrorrepo \
+     pull origin main --require-static-deltas 2>err.txt; then
+  assert_not_reached "--require-static-deltas unexpectedly succeeded"
+fi
+assert_file_has_content err.txt "Can't use static deltas in an archive repo"
+${CMD_PREFIX} ostree --repo=mirrorrepo pull origin main
+${CMD_PREFIX} ostree --repo=mirrorrepo fsck
+echo "ok pull (refuses deltas)"
+
 cd ${test_tmpdir}
 rm mirrorrepo/refs/remotes/* -rf
 ${CMD_PREFIX} ostree --repo=mirrorrepo prune --refs-only

diff --git a/tests/test-delta.sh b/tests/test-delta.sh
index 6d041f722af72ba7a1074f6df6c52cacd12ae737..46af2582adbb4edd3f6b034f1a63320700197db9 100755 (executable)
--- a/tests/test-delta.sh
+++ b/tests/test-delta.sh
@@ -168,7 +168,7 @@ echo 'ok heuristic endian detection'
 
 ${CMD_PREFIX} ostree --repo=repo summary -u
 
-mkdir repo2 && ${CMD_PREFIX} ostree --repo=repo2 init --mode=archive-z2
+mkdir repo2 && ${CMD_PREFIX} ostree --repo=repo2 init --mode=bare-user
 ${CMD_PREFIX} ostree --repo=repo2 pull-local --require-static-deltas repo ${newrev}
 ${CMD_PREFIX} ostree --repo=repo2 fsck
 ${CMD_PREFIX} ostree --repo=repo2 ls ${newrev} >/dev/null
@@ -236,7 +236,7 @@ echo 'ok generate + show empty delta part'
 ${CMD_PREFIX} ostree --repo=repo summary -u
 
 rm -rf repo2
-mkdir repo2 && ${CMD_PREFIX} ostree --repo=repo2 init --mode=archive-z2
+mkdir repo2 && ${CMD_PREFIX} ostree --repo=repo2 init --mode=bare-user
 ${CMD_PREFIX} ostree --repo=repo2 pull-local repo ${newrev}
 ${CMD_PREFIX} ostree --repo=repo2 pull-local --require-static-deltas repo ${samerev}
 ${CMD_PREFIX} ostree --repo=repo2 fsck